popupMenu: make submenus scrollable if needed

Right now, the network menu will overflow the screen if More...
is selected with many access points. As a short-term workaround
for this, add a scrollbar for submenus of panel dropdown menus
if they would cause the toplevel menu to overflow the screen.

- Put the actors in a PopupSubMenu in a StScrollView so we get
  a scrollbar if the allocated space is smaller than the height
  of the menu. Expand animation is turned off in the scrolled case
  to avoid weirdness.
- When we pop up a panel menu, set a max-height style property
  on the panel menu to limit it to the height of the screen.
- Hack event handling while the scrollbar is dragged to make
  the scrollbar work properly.

https://bugzilla.gnome.org/show_bug.cgi?id=646001

diff --git a/js/ui/panelMenu.js b/js/ui/panelMenu.js
index 6433433f785bbfd8232fd5f561bb38f6029ef3ea..cb5088060314a8e28254d288e91a254782948e87 100644 (file)
--- a/js/ui/panelMenu.js
+++ b/js/ui/panelMenu.js
@@ -32,6 +32,15 @@ Button.prototype = {
     },
 
     _onButtonPress: function(actor, event) {
+        if (!this.menu.isOpen) {
+            // Setting the max-height won't do any good if the minimum height of the
+            // menu is higher then the screen; it's useful if part of the menu is
+            // scrollable so the minimum height is smaller than the natural height
+            let monitor = global.get_primary_monitor();
+            this.menu.actor.style = ('max-height: ' +
+                                     Math.round(monitor.height - Main.panel.actor.height) +
+                                     'px;');
+        }
         this.menu.toggle();
     },
 

diff --git a/js/ui/popupMenu.js b/js/ui/popupMenu.js
index 1f31c18a4f5eed358fc86b41f755722d0c04c2c2..2bc388fdb2e824c4e6120133da86bc82f6ddd625 100644 (file)
--- a/js/ui/popupMenu.js
+++ b/js/ui/popupMenu.js
@@ -773,6 +773,10 @@ PopupMenuBase.prototype = {
         // for the menu which causes its prelight state to freeze
         this.blockSourceEvents = false;
 
+        // Can be set while a menu is up to let all events through without special
+        // menu handling useful for scrollbars in menus, and probably not otherwise.
+        this.passEvents = false;
+
         this._activeMenuItem = null;
     },
 
@@ -1043,40 +1047,103 @@ PopupSubMenu.prototype = {
         this._arrow = sourceArrow;
         this._arrow.rotation_center_z_gravity = Clutter.Gravity.CENTER;
 
-        this.actor = this.box;
+        // Since a function of a submenu might be to provide a "More.." expander
+        // with long content, we make it scrollable - the scrollbar will only take
+        // effect if a CSS max-height is set on the top menu.
+        this.actor = new St.ScrollView({ hscrollbar_policy: Gtk.PolicyType.NEVER,
+                                         vscrollbar_policy: Gtk.PolicyType.NEVER });
+
+        // StScrollbar plays dirty tricks with events, calling
+        // clutter_set_motion_events_enabled (FALSE) during the scroll; this
+        // confuses our event tracking, so we just turn it off during the
+        // scroll.
+        let vscroll = this.actor.get_vscroll_bar();
+        vscroll.connect('scroll-start',
+                        Lang.bind(this, function() {
+                                      let topMenu = this._getTopMenu();
+                                      if (topMenu)
+                                          topMenu.passEvents = true;
+                                  }));
+        vscroll.connect('scroll-stop',
+                        Lang.bind(this, function() {
+                                      let topMenu = this._getTopMenu();
+                                      if (topMenu)
+                                          topMenu.passEvents = false;
+                                  }));
+
+        this.actor.add_actor(this.box);
         this.actor._delegate = this;
         this.actor.clip_to_allocation = true;
         this.actor.connect('key-press-event', Lang.bind(this, this._onKeyPressEvent));
         this.actor.hide();
     },
 
+    _getTopMenu: function() {
+        let actor = this.actor.get_parent();
+        while (actor) {
+            if (actor._delegate && actor._delegate instanceof PopupMenu)
+                return actor._delegate;
+
+            actor = actor.get_parent();
+        }
+
+        return null;
+    },
+
+    _needsScrollbar: function() {
+        let topMenu = this._getTopMenu();
+        let [topMinHeight, topNaturalHeight] = topMenu.actor.get_preferred_height(-1);
+        let topThemeNode = topMenu.actor.get_theme_node();
+
+        let topMaxHeight = topThemeNode.get_max_height();
+        return topMaxHeight >= 0 && topNaturalHeight >= topMaxHeight;
+    },
+
     open: function(animate) {
         if (this.isOpen)
             return;
 
         this.isOpen = true;
 
-        // we don't implement the !animate case because that doesn't
-        // currently get used...
-
         this.actor.show();
-        let [naturalHeight, minHeight] = this.actor.get_preferred_height(-1);
-        this.actor.height = 0;
-        this.actor._arrow_rotation = this._arrow.rotation_angle_z;
-        Tweener.addTween(this.actor,
-                         { _arrow_rotation: 90,
-                           height: naturalHeight,
-                           time: 0.25,
-                           onUpdateScope: this,
-                           onUpdate: function() {
-                               this._arrow.rotation_angle_z = this.actor._arrow_rotation;
-                           },
-                           onCompleteScope: this,
-                           onComplete: function() {
-                               this.actor.set_height(-1);
-                               this.emit('open-state-changed', true);
-                           }
-                         });
+
+        let needsScrollbar = this._needsScrollbar();
+
+        // St.ScrollView always requests space horizontally for a possible vertical
+        // scrollbar if in AUTOMATIC mode. Doing better would require implementation
+        // of width-for-height in St.BoxLayout and St.ScrollView. This looks bad
+        // when we *don't* need it, so turn off the scrollbar when that's true.
+        // Dynamic changes in whether we need it aren't handled properly.
+        this.actor.vscrollbar_policy =
+            needsScrollbar ? Gtk.PolicyType.AUTOMATIC : Gtk.PolicyType.NEVER;
+
+        // It looks funny if we animate with a scrollbar (at what point is
+        // the scrollbar added?) so just skip that case
+        if (animate && needsScrollbar)
+            animate = false;
+
+        if (animate) {
+            let [minHeight, naturalHeight] = this.actor.get_preferred_height(-1);
+            this.actor.height = 0;
+            this.actor._arrow_rotation = this._arrow.rotation_angle_z;
+            Tweener.addTween(this.actor,
+                             { _arrow_rotation: 90,
+                               height: naturalHeight,
+                               time: 0.25,
+                               onUpdateScope: this,
+                               onUpdate: function() {
+                                   this._arrow.rotation_angle_z = this.actor._arrow_rotation;
+                               },
+                               onCompleteScope: this,
+                               onComplete: function() {
+                                   this.actor.set_height(-1);
+                                   this.emit('open-state-changed', true);
+                               }
+                             });
+        } else {
+            this._arrow.rotation_angle_z = 90;
+            this.emit('open-state-changed', true);
+        }
     },
 
     close: function(animate) {
@@ -1088,6 +1155,9 @@ PopupSubMenu.prototype = {
         if (this._activeMenuItem)
             this._activeMenuItem.setActive(false);
 
+        if (animate && this._needsScrollbar())
+            animate = false;
+
         if (animate) {
             this.actor._arrow_rotation = this._arrow.rotation_angle_z;
             Tweener.addTween(this.actor,
@@ -1423,8 +1493,12 @@ PopupMenuManager.prototype = {
             this._owner.menuEventFilter(event))
             return true;
 
+        if (this._activeMenu != null && this._activeMenu.passEvents)
+            return false;
+
         let activeMenuContains = this._eventIsOnActiveMenu(event);
         let eventType = event.type();
+
         if (eventType == Clutter.EventType.BUTTON_RELEASE) {
             if (activeMenuContains) {
                 return false;